Data flow: Track call contexts in parameterValueFlow#17876
Merged
hvitved merged 3 commits intogithub:mainfrom Nov 21, 2024
Merged
Data flow: Track call contexts in parameterValueFlow#17876hvitved merged 3 commits intogithub:mainfrom
parameterValueFlow#17876hvitved merged 3 commits intogithub:mainfrom
Conversation
hvitved
changed the title
parameterFlowparameterValueFlow
hvitved
force-pushed
the
dataflow/param-flow-call-ctx
branch
from
6dd83e4 to
9037ff0
Compare
aschackmull
reviewed
Nov 11, 2024
Comment on lines
+1443
to
+1448
| // call may restrict the set of call sites that can be returned to | ||
| ctx2.(CachedCallContextSensitivity::CcReturn).isReturn(callable, call) | ||
| or | ||
| // call does not restrict the set of call sites that can be returned to | ||
| not exists(CachedCallContextSensitivity::CcReturn ret | ret.isReturn(callable, call)) and | ||
| CachedCallContextSensitivity::viableImplNotCallContextReducedReverse(ctx2) |
Contributor
There was a problem hiding this comment.
This calculation that derives ctx2 from the (callable, call) pair is a deflating calculation, so it'll be more efficient to do it earlier. We can push it all the way into argumentValueFlowsThrough0.
Contributor
There was a problem hiding this comment.
Also, it already exists as a bindingset-annotated predicate: getCallContextReturn, so no need to duplicate that logic.
aschackmull
reviewed
Nov 11, 2024
Contributor
aschackmull
left a comment
aschackmull
left a comment
There was a problem hiding this comment.
One simplification suggested, otherwise LGTM.
hvitved
force-pushed
the
dataflow/param-flow-call-ctx
branch
from
9037ff0 to
045d424
Compare
aschackmull
reviewed
Nov 20, 2024
| pragma[nomagic] | ||
| private predicate argumentValueFlowsThrough0( | ||
| DataFlowCall call, ArgNode arg, ReturnKind kind, ReadStepTypesOption read, string model | ||
| DataFlowCall call, DataFlowCallable callable, ArgNode arg, ReturnKind kind, |
Contributor
There was a problem hiding this comment.
The callable column is unused and can be pushed into the exists.
aschackmull
reviewed
Nov 20, 2024
|
|
||
| pragma[nomagic] | ||
| private predicate argumentValueFlowsThrough( | ||
| DataFlowCall call, DataFlowCallable callable, ArgNode arg, ReadStepTypesOption read, |
Contributor
There was a problem hiding this comment.
The call column can be pushed into the exists here.
aschackmull
reviewed
Nov 20, 2024
| exists(DataFlowCall call | this = TReturn(_, call) | result = "CcReturn(" + call + ")") | ||
| } | ||
|
|
||
| predicate isReturn(DataFlowCallable c, DataFlowCall call) { this = TReturn(c, call) } |
Contributor
There was a problem hiding this comment.
This can be removed now.
hvitved
force-pushed
the
dataflow/param-flow-call-ctx
branch
from
045d424 to
fc1bc3a
Compare
hvitved
force-pushed
the
dataflow/param-flow-call-ctx
branch
from
fc1bc3a to
3f56fc9
Compare
aschackmull
approved these changes
Nov 20, 2024
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
When investigating a Ruby performance issue, I found that we produced some false positive return-through-parameter flow, which is because we are not tracking call contexts in the
parameterValueFlowlogic. This PR adds tracking of call contexts.